chore: split `config.Registries` into the separate resource. #9780

DmitriyMV · 2024-11-21T19:46:56Z

Required for #9614

Closes #9766

dsseng

Maybe this could also remove those config definitions from the old place as well?

pkg/machinery/resources/registry/registry.go

internal/app/machined/pkg/controllers/files/cri_registry_config.go

smira

the image/resolver (via PullImage) hasn't been updated

DmitriyMV · 2024-11-26T08:58:00Z

@smira I think all paths leading to image package are updated now.

smira · 2024-11-26T10:31:25Z

hack/structprotogen/proto/proto.go

+		// This is a hack, but I (Dmitry) don't have enough patience to figure out why we don't support complex maps
+		return "resource/definitions/cri/registry.proto", "talos.resource.definitions.cri.RegistryMirrorConfig"
+	case typeData{"github.com/siderolabs/talos/pkg/machinery/resources/cri", "RegistryConfig"}:
+		// This is a hack, but I (Dmitry) don't have enough patience to figure out why we don't support complex maps


the author gave up 😛

pkg/machinery/resources/cri/registry.go

internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go

DmitriyMV · 2024-11-26T10:59:05Z

Integration provision passes, so upgrades work. I'll remove the label to speed-up process.

smira

lots of other fixes here as we went through testing image cache

smira · 2024-11-26T16:22:23Z

Missing bits:

some simple unit-test for RegistriesConfig controller - it generate empty on no config, it generates something with config, it enables stuff with image cache config
some unit-tests for registryd itself (might be super high-level, using httptest or lower level)

frezbo · 2024-11-26T17:01:30Z

lots of other fixes here as we went through testing image cache

should we split those into a different PR, easier to backport if needed

smira · 2024-11-26T17:20:01Z

lots of other fixes here as we went through testing image cache

should we split those into a different PR, easier to backport if needed

this also works, probably halt_if_installed is the only important fix that can be backported

internal/app/machined/pkg/controllers/cri/registry.go

DmitriyMV · 2024-11-27T15:36:16Z

Updated PR:

Print errors to the zap logger (and optionally return from the iterator) during file system iterator call.
Add minimal unit-tests for cri.NewRegistriesConfigController and registry.Service.
Return proper path errors from registry.MultiPathFS Open and Stat methods.

smira · 2024-11-27T15:41:48Z

internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go

@@ -127,13 +128,11 @@ func (s *Server) checkSupported(feature runtime.ModeCapability) error {

 func (s *Server) checkControlplane(apiName string) error {
 	switch s.Controller.Runtime().Config().Machine().Type() { //nolint:exhaustive


if touching this, better to refactor using .IsControlplane() method of the machine.Type

Dropped all irrelevant stuff.

smira · 2024-11-27T15:43:26Z

internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go

@@ -1228,19 +1227,19 @@ func (s *Server) Logs(req *machine.LogsRequest, l machine.MachineService_LogsSer

 	switch {
 	case req.Namespace == constants.SystemContainerdNamespace || req.Id == "kubelet":
-		var options []runtime.LogOption
+		var opts []runtime.LogOption


can we please skip in the future changing every line for unrelated changes, and close to beta release specifically. it makes more work for reviewers.

Dropped all irrelevant stuff.

I've already spent time reviewing it, so now it was wasted... 😭

Sorry! I thought you wanted me to drop this unrelated stuff.

smira · 2024-11-27T15:44:00Z

internal/app/machined/pkg/controllers/cri/registry.go

@@ -0,0 +1,118 @@
+// This Source Code Form is subject to the terms of the Mozilla Public


the file should be registries_config.go to match controller name

smira · 2024-11-27T15:44:47Z

internal/app/machined/pkg/controllers/cri/registry_test.go

+}
+
+func (suite *ConfigSuite) TestRegistry() {
+	cfg := config.NewMachineConfig(container.NewV1Alpha1(&v1alpha1.Config{


there's a missing case for missing machine config

Added TestRegistryNoMachineConfig.

smira · 2024-11-27T15:45:41Z

internal/app/machined/pkg/system/services/registry/app/main.go

@@ -38,9 +37,9 @@ func app() error {
 		return fmt.Errorf("failed to get user home directory: %w", err)
 	}

-	it := func(yield func(fs.StatFS) bool) {
+	it := func(yield func(string2 string) bool) {


string2 ? or should be path?

It should be nothing, but IDE couldnt figure that out and inserted a placeholder name. Removed.

Required for siderolabs#9614 Closes siderolabs#9766 Signed-off-by: Dmitriy Matrenichev <[email protected]>

DmitriyMV · 2024-11-27T18:16:19Z

/m

talos-bot added the status/ok-to-test label Nov 21, 2024

DmitriyMV force-pushed the split-registry branch from bac4314 to afa039e Compare November 21, 2024 19:48

DmitriyMV changed the title ~~chore: split config.Registry into the separate resource.~~ chore: split config.Registries into the separate resource. Nov 21, 2024

DmitriyMV force-pushed the split-registry branch from afa039e to b7bb576 Compare November 21, 2024 19:55

dsseng approved these changes Nov 21, 2024

View reviewed changes

smira reviewed Nov 25, 2024

View reviewed changes

pkg/machinery/resources/registry/registry.go Outdated Show resolved Hide resolved

smira reviewed Nov 25, 2024

View reviewed changes

internal/app/machined/pkg/controllers/files/cri_registry_config.go Outdated Show resolved Hide resolved

smira reviewed Nov 25, 2024

View reviewed changes

DmitriyMV force-pushed the split-registry branch from b7bb576 to d0f1a62 Compare November 26, 2024 08:51

DmitriyMV added the integration/provision label Nov 26, 2024

smira reviewed Nov 26, 2024

View reviewed changes

pkg/machinery/resources/cri/registry.go Outdated Show resolved Hide resolved

smira reviewed Nov 26, 2024

View reviewed changes

pkg/machinery/resources/cri/registry.go Outdated Show resolved Hide resolved

smira reviewed Nov 26, 2024

View reviewed changes

internal/app/machined/pkg/runtime/v1alpha1/v1alpha1_sequencer_tasks.go Outdated Show resolved Hide resolved

DmitriyMV force-pushed the split-registry branch from d0f1a62 to 65d4127 Compare November 26, 2024 10:57

DmitriyMV removed the integration/provision label Nov 26, 2024

DmitriyMV force-pushed the split-registry branch from 65d4127 to 3ffb2e1 Compare November 26, 2024 11:03

smira force-pushed the split-registry branch 2 times, most recently from 3837e2b to 1f4bee3 Compare November 26, 2024 15:02

smira approved these changes Nov 26, 2024

View reviewed changes

smira reviewed Nov 27, 2024

View reviewed changes

internal/app/machined/pkg/controllers/cri/registry.go Outdated Show resolved Hide resolved

DmitriyMV force-pushed the split-registry branch from 1f4bee3 to 897dc60 Compare November 27, 2024 15:31

DmitriyMV force-pushed the split-registry branch from 897dc60 to 04eed6f Compare November 27, 2024 15:39

smira reviewed Nov 27, 2024

View reviewed changes

smira approved these changes Nov 27, 2024

View reviewed changes

DmitriyMV force-pushed the split-registry branch 2 times, most recently from e040bbb to 1e20ea8 Compare November 27, 2024 16:33

chore: split config.Registry into the separate resource

ccc5a8d

Required for siderolabs#9614 Closes siderolabs#9766 Signed-off-by: Dmitriy Matrenichev <[email protected]>

DmitriyMV force-pushed the split-registry branch from 1e20ea8 to ccc5a8d Compare November 27, 2024 17:39

talos-bot added the status/ok-to-merge label Nov 27, 2024

talos-bot merged commit ccc5a8d into siderolabs:main Nov 27, 2024
50 checks passed

talos-bot removed status/ok-to-test status/ok-to-merge labels Nov 27, 2024

DmitriyMV deleted the split-registry branch November 27, 2024 18:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: split `config.Registries` into the separate resource. #9780

chore: split `config.Registries` into the separate resource. #9780

DmitriyMV commented Nov 21, 2024

dsseng left a comment

smira left a comment

DmitriyMV commented Nov 26, 2024

smira Nov 26, 2024

DmitriyMV Nov 26, 2024

DmitriyMV commented Nov 26, 2024

smira left a comment

smira commented Nov 26, 2024

frezbo commented Nov 26, 2024

smira commented Nov 26, 2024

DmitriyMV commented Nov 27, 2024 •

edited

Loading

smira Nov 27, 2024

DmitriyMV Nov 27, 2024

smira Nov 27, 2024

DmitriyMV Nov 27, 2024

smira Nov 27, 2024

DmitriyMV Nov 27, 2024

smira Nov 27, 2024

DmitriyMV Nov 27, 2024

smira Nov 27, 2024

DmitriyMV Nov 27, 2024

smira Nov 27, 2024

DmitriyMV Nov 27, 2024 •

edited

Loading

DmitriyMV commented Nov 27, 2024

		@@ -127,13 +128,11 @@ func (s *Server) checkSupported(feature runtime.ModeCapability) error {

		func (s *Server) checkControlplane(apiName string) error {
		switch s.Controller.Runtime().Config().Machine().Type() { //nolint:exhaustive

		@@ -0,0 +1,118 @@
		// This Source Code Form is subject to the terms of the Mozilla Public

chore: split config.Registries into the separate resource. #9780

chore: split config.Registries into the separate resource. #9780

Conversation

DmitriyMV commented Nov 21, 2024

dsseng left a comment

Choose a reason for hiding this comment

smira left a comment

Choose a reason for hiding this comment

DmitriyMV commented Nov 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyMV commented Nov 26, 2024

smira left a comment

Choose a reason for hiding this comment

smira commented Nov 26, 2024

frezbo commented Nov 26, 2024

smira commented Nov 26, 2024

DmitriyMV commented Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyMV Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

DmitriyMV commented Nov 27, 2024

chore: split `config.Registries` into the separate resource. #9780

chore: split `config.Registries` into the separate resource. #9780

DmitriyMV commented Nov 27, 2024 •

edited

Loading

DmitriyMV Nov 27, 2024 •

edited

Loading